Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Conceal serde types with Swift SPI #805

Merged
merged 11 commits into from
Sep 3, 2024
Merged

Conversation

jbelkins
Copy link
Contributor

@jbelkins jbelkins commented Aug 29, 2024

Issue #

awslabs/aws-sdk-swift#1702

Description of changes

Conceals the public interfaces related to serde using the Swift @_spi() annotation.
This makes clear to customers that though public, the serde types are not intended for them to interact with directly.

Types that are now SPI:

  • SmithyReader, SmithyWriter and all implementations of those types.
  • All reading & writing closures, and their typealias definitions.
  • All document reading & writing closures, and their typealias definitions.
  • The entire SmithyTimestamps module.
  • A couple middlewares that have serde types in their interfaces.

Also:

  • The SwiftSymbol factory method is improved with new methods for importing additional symbols and annotating a symbol with more than one SPI.
  • The serde extension on the ByteStream type is removed from ClientRuntime and moved directly into SmithyReadWrite.

Scope

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -103,7 +103,7 @@ let package = Package(
),
.target(
name: "SmithyReadWrite",
dependencies: ["SmithyTimestamps"]
dependencies: ["Smithy", "SmithyTimestamps"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ByteStream is now in the Smithy module, SmithyReadWrite takes a dependency on Smithy and provides its own read-write implementation for ByteStream.

@@ -7,9 +7,10 @@

import class SmithyHTTPAPI.HTTPResponse
Copy link
Contributor Author

@jbelkins jbelkins Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below you will see that many declarations and imports of those declarations are tagged with @_spi(...) to prevent them from showing up as public API.

I will not comment on the rest of these type of changes below.

import enum Smithy.ByteStream

extension SmithyWriter {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method below was moved into SmithyReadWrite.

@@ -47,6 +49,17 @@ public protocol SmithyWriter: AnyObject {

public extension SmithyWriter {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function below was moved here from ClientRuntime since ByteStream now resides in Smithy module.

@@ -206,6 +207,7 @@ class MessageUnmarshallableGenerator(

private fun renderReadToValue(writer: SwiftWriter, memberShape: MemberShape) {
val readingClosure = ReadingClosureUtils(ctx, writer).readingClosure(memberShape)
writer.addImport(SmithyReadWriteTypes.SmithyReader)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An import of SmithyReader is added here because some of the methods being used are SPI methods extended on that type.

@@ -83,6 +85,7 @@ class HTTPResponseBindingErrorGenerator(
.map { ctx.model.expectShape(it) as StructureShape }
.toSet()
.sorted()
writer.addImport(SwiftSymbol.make("ClientRuntime", null, SwiftDependency.CLIENT_RUNTIME, "SmithyReadWrite"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An import of all of ClientRuntime with the SmithyReadWrite SPI annotation is required to access the HTTPResponse.data() instance method.

@@ -50,6 +52,8 @@ class HTTPResponseBindingErrorInitGenerator(
errorShape,
) {
if (needsReader) {
writer.addImport(SmithyReadWriteTypes.SmithyReader)
writer.addImport(ctx.service.readerSymbol)
Copy link
Contributor Author

@jbelkins jbelkins Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import ensures that all the SPI methods on the reader are exposed.

@@ -14,22 +14,19 @@ class SwiftImportContainer : ImportContainer {
fun addImport(
packageName: String,
isTestable: Boolean = false,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below, the import container is modified to handle more than one SPI name.

@@ -104,7 +105,7 @@ class SwiftWriter(
fun addImport(symbol: Symbol) {
symbol.references.forEach { addImport(it.symbol) }
if (symbol.isBuiltIn || symbol.isServiceNestedNamespace || symbol.namespace.isEmpty()) return
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below, SwiftWriter is modified to handle symbols that have multiple SPI names.

}
symbol.dependencies.forEach { addDependency(it) }
val additionalImports = symbol.getProperty("additionalImports").getOrElse { emptyList<Symbol>() } as List<Symbol>
additionalImports.forEach { addImport(it) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional imports are read from the symbol, and each is imported itself.

@@ -83,6 +85,7 @@ class HTTPResponseBindingErrorGenerator(
.map { ctx.model.expectShape(it) as StructureShape }
.toSet()
.sorted()
writer.addImport(SwiftSymbol.make("ClientRuntime", null, SwiftDependency.CLIENT_RUNTIME, emptyList(), listOf("SmithyReadWrite")))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is needed to expose the .data() extension on HTTPResponse.

@@ -60,6 +62,7 @@ class HTTPResponseBindingOutputGenerator(
writer.write("return \$N()", outputSymbol)
} else {
if (needsAReader(ctx, responseBindings)) {
writer.addImport(SwiftSymbol.make("ClientRuntime", null, SwiftDependency.CLIENT_RUNTIME, emptyList(), listOf("SmithyReadWrite")))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like on the error init generator above, this import exposes .data() on HTTPResponse.

@@ -118,10 +119,11 @@ open class MemberShapeDecodeGenerator(
val memberTimestampFormatTrait = memberShape.getTrait<TimestampFormatTrait>()
val swiftTimestampFormatCase = TimestampUtils.timestampFormat(ctx, memberTimestampFormatTrait, timestampShape)
return writer.format(
"try \$L.\$L(format: \$L)",
"try \$L.\$L(format: \$N\$L)",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TimestampFormat type is explicitly referenced so it gets the needed SPI import.

The same thing happens below where a timestamp format is rendered.

@@ -23,7 +25,7 @@ object ClientRuntimeTypes {
val ContentLengthMiddleware = runtimeSymbol("ContentLengthMiddleware", SwiftDeclaration.STRUCT)
val ContentTypeMiddleware = runtimeSymbol("ContentTypeMiddleware", SwiftDeclaration.STRUCT)
val ContentMD5Middleware = runtimeSymbol("ContentMD5Middleware", SwiftDeclaration.STRUCT)
val DeserializeMiddleware = runtimeSymbol("DeserializeMiddleware", SwiftDeclaration.STRUCT)
val DeserializeMiddleware = runtimeSymbol("DeserializeMiddleware", SwiftDeclaration.STRUCT, emptyList(), listOf("SmithyReadWrite"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeserializeMiddleware and BodyMiddleware are marked SPI because they reference SPI types in their interfaces.

SwiftDependency.CLIENT_RUNTIME,
null,
)
object Composite {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two symbols are "composite symbols", meaning that they reference other symbols in their definition, in this case the two symbols are arrays where another symbol is the element type.

The new "additionalImports" property on SwiftSymbol is used to ensure the inner types get imported as needed.


private fun runtimeSymbolWithoutNamespace(name: String, declaration: SwiftDeclaration? = null): Symbol = SwiftSymbol.make(
private fun runtimeSymbol(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is modified to allow for multiple SPIs on a symbol, and to allow for additional imports.

@@ -9,12 +9,17 @@ import software.amazon.smithy.swift.codegen.SwiftDeclaration
import software.amazon.smithy.swift.codegen.SwiftDependency

object SmithyFormURLTypes {
val Writer = runtimeSymbol("Writer", SwiftDeclaration.CLASS)
val Writer = runtimeSymbol("Writer", SwiftDeclaration.CLASS, listOf(SmithyReadWriteTypes.SmithyWriter))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FormURLWriter is made SPI, and imports the SmithyWriter protocol along with itself.

val Writer = runtimeSymbol("Writer", SwiftDeclaration.CLASS)
val Reader = runtimeSymbol("Reader", SwiftDeclaration.CLASS)
val Writer = runtimeSymbol("Writer", SwiftDeclaration.CLASS, listOf(SmithyReadWriteTypes.SmithyWriter))
val Reader = runtimeSymbol("Reader", SwiftDeclaration.CLASS, listOf(SmithyReadWriteTypes.SmithyReader))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smithy JSON Reader & Writer are made SPI and import SmithyReader / SmithyWriter along with themselves.

val SmithyReader = runtimeSymbol("SmithyReader", SwiftDeclaration.PROTOCOL)
val SmithyWriter = runtimeSymbol("SmithyWriter", SwiftDeclaration.PROTOCOL)
val Document = runtimeSymbol("Document", SwiftDeclaration.ENUM, emptyList())
val ReaderError = runtimeSymbol("ReaderError", SwiftDeclaration.ENUM, emptyList())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All SmithyReadWrite symbols are made SPI except for Document and ReaderError.

val Writer = runtimeSymbol("Writer", SwiftDeclaration.CLASS)
val Reader = runtimeSymbol("Reader", SwiftDeclaration.CLASS)
val Writer = runtimeSymbol("Writer", SwiftDeclaration.CLASS, listOf(SmithyReadWriteTypes.SmithyWriter))
val Reader = runtimeSymbol("Reader", SwiftDeclaration.CLASS, listOf(SmithyReadWriteTypes.SmithyReader))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SmithyXML Reader & Writer import SmithyReader & SmithyWriter along with themselves.

Also, all SmithyXML types are made SPI.

@@ -12,11 +12,13 @@ class SwiftSymbol {
name: String,
declaration: SwiftDeclaration?,
dependency: Dependency?,
spiName: String?
additionalImports: List<Symbol>,
spiNames: List<String>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The factory method in SwiftSymbol adds parameters for additional imports, and changes the spiName import from an optional String to a list of Strings.

@@ -95,12 +95,12 @@ extension TimestampInputInput {

static func write(value: TimestampInputInput?, to writer: SmithyJSON.Writer) throws {
guard let value else { return }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below, many codegen tests have changed to expect an explicit type on timestamp format.

@jbelkins jbelkins changed the title Jbe/smithyreadwrite spi feat!: Conceal serde types with Swift SPI Aug 30, 2024
@jbelkins jbelkins marked this pull request as ready for review August 30, 2024 19:44
@jbelkins jbelkins merged commit 2936b2f into main Sep 3, 2024
27 checks passed
@jbelkins jbelkins deleted the jbe/smithyreadwrite_spi branch September 3, 2024 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants